Overhaul Apache RAT by dsmiley · Pull Request #4569 · apache/solr

dsmiley · 2026-06-29T16:17:41Z

Upgrade from 0.15 to 0.18
Use a config file
Remove custom license patterns that match nothing anymore
Ensure works even without a git working tree (e.g. smoketester source release)
Add a canary to assert that header detection is actually working each run
Synchronize RatTask executions to avoid shared Ant task state issues under parallel Gradle execution
Consolidate all exclude patterns into a single include-everything-then-exclude approach

Note: requires the PR that excludes dev-docs and dot-files from the source release, in order for RAT to pass on smoketest release

* Upgrade from 0.15 to 0.18 * Use a config file * Remove custom license patterns that match nothing anymore * Ensure works even without a git working tree (e.g. smoketester source release) * Add a canary to assert that header detection is actually working each run * Synchronize RatTask executions to avoid shared Ant task state issues under parallel Gradle execution * Consolidate all exclude patterns into a single include-everything-then-exclude approach

dsmiley

I chickened out and didn't pull out jgit... but was tempted.

dsmiley · 2026-06-29T16:18:51Z

Instead of tying this to RAT, the needs are too simple/basic to warrant that coupling.

dsmiley · 2026-06-29T16:20:13Z

+        exclude "**/.*" // e.g. .gradle, .github, .junie, .gitignore, etc.
+      } else {
+        // Source release (no git): scan everything, exclude generated/ignored paths manually.
+        include "**"


key change here, esp. for the source release validation (smoketester). Include all by default.

Claude had a comment on this change:

.gradle/ is no longer excluded in the no-git path (the old code excluded .gradle/** unconditionally; the new **/.* exclude only lives inside the if (trackedFiles != null) branch). In an extracted source release the running Gradle invocation itself creates .gradle/ and buildSrc/.gradle with files like checksums.lock and file-system.probe that the extension-based excludes don't catch — RAT could flag them during smoketesting.

The no-git path is is the source release, tested by the smoketester. I was tempted to make that the only path for RAT, albeit it'd suck to get no PR time feedback on an oversight. So for the source release... we can ensure that the smoketester runs RAT by itself before any other source checking... thus nothing is generated yet. But I suppose .gradle is a concern... hmm; I will add an exclusion rule for that.

dsmiley · 2026-06-29T16:20:32Z

-      // TODO: SOLR-15601: Some of these should carry the license, perhaps?
-      exclude "**/*.html"
+      exclude subprojects.collect {
+        projectDir.toPath().relativize(it.projectDir.toPath()).toString().replace('\\', '/')


was overly simple before, thus would exclude any "core" dir anywhere for example

dsmiley · 2026-07-01T22:03:54Z

I'll merge this weekend if no feedback.

I did some testing of course... this is also what motivated the "canary" aspect. I kinda tested smoketester but had some local edits in relation to local/snapshot releases so I can't guarantee bug-free

janhoy

Thanks for caring for boring stuff like this. Added some comments for stuff flagged by claude

janhoy · 2026-07-03T12:39:48Z

+        exclude "**/.*" // e.g. .gradle, .github, .junie, .gitignore, etc.
+      } else {
+        // Source release (no git): scan everything, exclude generated/ignored paths manually.
+        include "**"


Claude had a comment on this change:

.gradle/ is no longer excluded in the no-git path (the old code excluded .gradle/** unconditionally; the new **/.* exclude only lives inside the if (trackedFiles != null) branch). In an extracted source release the running Gradle invocation itself creates .gradle/ and buildSrc/.gradle with files like checksums.lock and file-system.probe that the extension-based excludes don't catch — RAT could flag them during smoketesting.

janhoy · 2026-07-03T12:40:39Z

+    <!-- Machine-generated source files that carry no license header -->
+    <license family="GEN" id="GEN" name="Generated">
+      <any>
+        <!-- none so far -->


Why include it if there are no rules?

There used to be some but all were old or part of Lucene. I thought it best to leave this here ready to easily add one if needed; it's minor. Of course, we actually do have generated source but I haven't looked at why we don't need this. For some I known we exclude categorically because it's not committed. For committed like a query parser or two, I think it's intermixed with human authored stuff so it gets a license. I think.

janhoy · 2026-07-03T12:43:11Z

+    // Detects whether a comment block contains Apache license header text.
+    def isLicense = { matcher ->
+      def content = matcher.group(1)
+      content.contains('Licensed to the Apache Software Foundation') ||


A comment by claude that I have not been able to validate but here it is:

The new string-based isLicense only recognizes Apache header text, while the old RAT default matchers also caught MIT/BSD-style javadoc headers. Coverage narrows slightly; probably an acceptable trade-off

indeed; acceptable trade-off. Prefer to keep things simple.

# Conflicts: # gradle.lockfile # gradle/libs.versions.toml

This option (PR-apache#2685, ported from a similar Lucene feature) added complexity: parallel lists of alt-JDK homes/runners/versions threaded through the Java config, a closure to dedupe per-JDK test runs, and per-JDK copies of the unpacked binary distribution. It was never well exercised -- it merged via lazy consensus with the sole tester noting they hadn't actually tried it with a different JDK version -- and it isn't invoked anywhere in the documented release process (releaseWizard.yaml never passes it). It has caused at least two real bugs, including a TypeError crash when no alt JDK is given at all. Testing with another JDK is just as easy by setting JAVA_HOME and re-running the script; the modest cost of redoing some work is a fair trade for a much simpler script.

dsmiley · 2026-07-04T17:56:19Z

I'm going to incorporate here an explicit step to the smoketester to invoke RAT. The only issue with my attempts right now to do so is that the smoketester writes the output of these gradle invocations to log files in the present directory, which means RAT sees it and sees that the log file has no license. I'll put it into build/ instead.

* move logs to under build/ * move no-daemon to an env var, off of CLI

dsmiley · 2026-07-04T18:15:33Z

Including a removal of the alt-java thing... but that is a separate PR and should be done first.

# Conflicts: # dev-tools/scripts/smokeTestRelease.py

dsmiley · 2026-07-07T01:17:46Z

+  validateCmd = './gradlew rat'
+  print('    run "%s"' % validateCmd)
+  java.run_java(validateCmd, '%s/rat.log' % logDir)
+


run "rat". It was surprising we weren't already running this tool in the smoketester given it's role.

dsmiley · 2026-07-07T01:18:36Z

  if cygwin:
    java_home = subprocess.check_output('cygpath -u "%s"' % java_home, shell=True).decode('utf-8').strip()
-  cmd_prefix = 'export JAVA_HOME="%s" PATH="%s/bin:$PATH" JAVACMD="%s/bin/java"' % \
+  cmd_prefix = 'export JAVA_HOME="%s" PATH="%s/bin:$PATH" JAVACMD="%s/bin/java" GRADLE_OPTS="-Dorg.gradle.daemon=false"' % \


I set the gradle daemon setting here to remove the repetitiveness above

* Upgrade Apache RAT from 0.15 to 0.18 and drive it via a config file instead of scattered custom patterns. * Consolidate scanning to a single include-everything-then-exclude approach; drop stale patterns that no longer match anything. * Add a per-module canary path to verify header detection is actually working on every run. * Synchronize RatTask executions to avoid shared Ant task state issue under parallel Gradle execution. * Make RAT work even without a git working tree (e.g. smoketester source release). * smokeTestRelease.py: run RAT as part of the smoke test; move logs under build/ and no-daemon to an env var. (cherry picked from commit b8af94a)

dsmiley requested a review from janhoy June 29, 2026 16:17

github-actions Bot added dependencies Dependency upgrades tool:build admin-ui labels Jun 29, 2026

dsmiley commented Jun 29, 2026

View reviewed changes

dsmiley added no-changelog and removed admin-ui labels Jun 29, 2026

janhoy mentioned this pull request Jul 2, 2026

Update all non-major dependencies #4591

Merged

1 task

janhoy reviewed Jul 3, 2026

View reviewed changes

Exclude .gradle

7f40e42

github-actions Bot added the admin-ui label Jul 3, 2026

janhoy approved these changes Jul 3, 2026

View reviewed changes

dsmiley added 4 commits July 4, 2026 01:13

Merge branch 'main' into rat-overhaul

3e4e614

# Conflicts: # gradle.lockfile # gradle/libs.versions.toml

changelog

1806d65

Merge branch 'smokeTestRelease_removeAltJava' into rat-overhaul

65bfdb9

smokeTestRelease.py: run rat.

3e365d0

* move logs to under build/ * move no-daemon to an env var, off of CLI

github-actions Bot added the scripts label Jul 4, 2026

Merge branch 'main' into rat-overhaul

aedf8db

# Conflicts: # dev-tools/scripts/smokeTestRelease.py

dsmiley commented Jul 7, 2026

View reviewed changes

dsmiley merged commit b8af94a into apache:main Jul 7, 2026
5 checks passed

dsmiley deleted the rat-overhaul branch July 7, 2026 19:22

dsmiley added this to the 9.x milestone Jul 9, 2026

Uh oh!

Conversation

dsmiley commented Jun 29, 2026

Uh oh!

dsmiley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsmiley commented Jul 1, 2026

Uh oh!

janhoy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsmiley commented Jul 4, 2026

Uh oh!

dsmiley commented Jul 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants